Skip to content

Conversation

@rayaisaiah
Copy link
Contributor

@rayaisaiah rayaisaiah commented Jan 28, 2025

Reason for Change:
Adds a tool to validate cx network polices are safe to migrate from Azure NPM to Cilium.

Issue Fixed:

Requirements:

Notes:

@rayaisaiah rayaisaiah marked this pull request as ready for review February 1, 2025 00:47
@rayaisaiah rayaisaiah requested a review from a team as a code owner February 1, 2025 00:47
testing not done

update

added new check still debugging service check

fixed logic on services with allow all ingress polcies

added checks for allow all ingress policies

added checks for services with allow all policys with empty and label selectors
@rayaisaiah rayaisaiah force-pushed the isaiahraya/npm-cilium-migration-script branch from 34ac8d3 to 087b1c1 Compare February 1, 2025 00:52
Copy link
Contributor

@huntergregory huntergregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later can we unit test each check? For services, one UT for each case?

@huntergregory huntergregory added npm Related to NPM. linux labels Feb 3, 2025
Copy link
Contributor

@huntergregory huntergregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments on the service check

@rayaisaiah
Copy link
Contributor Author

/azp run Azure Container Networking PR

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

huntergregory
huntergregory previously approved these changes Feb 26, 2025
@rayaisaiah
Copy link
Contributor Author

/azp run Azure Container Networking PR

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

vakalapa
vakalapa previously approved these changes Feb 26, 2025
@rayaisaiah
Copy link
Contributor Author

/azp run Azure Container Networking PR

@rayaisaiah
Copy link
Contributor Author

/azp run Azure Container Networking PR

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rayaisaiah rayaisaiah added this pull request to the merge queue Feb 26, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 27, 2025
@rayaisaiah rayaisaiah added this pull request to the merge queue Feb 27, 2025
Merged via the queue into master with commit c697ff6 Feb 27, 2025
14 checks passed
@rayaisaiah rayaisaiah deleted the isaiahraya/npm-cilium-migration-script branch February 27, 2025 19:16
sivakami-projects pushed a commit that referenced this pull request Oct 23, 2025
* npm to cilium validator script

testing not done

update

added new check still debugging service check

fixed logic on services with allow all ingress polcies

added checks for allow all ingress policies

added checks for services with allow all policys with empty and label selectors

* added a check for services with target ports

* update for lint errors with repeat imports and using slice of pointers for service and policy maps

* made a function to reuse for Ingress and egress ports

* added some unit tests except for service check and made print statements log in order to test

* updated engress policy check with egress allow all policy and added a helper to test functions

* changed file path

* added namedport checks and added port to ingress check

* responded to service comments

* added a check for ingress deny all and updated port check function to be a nested loop

* updated to return lists and use pointers but still broken for services

* added pointers to service check functions

* fixed pointer logic and added unit tests for the checks except service

* fixed all linter errors

* updated difference function with comment to use a set

* fixed linter problems induced by previous commit

* added complete UTs for GetEndportNetworkPolicies, GetCIDRNetworkPolicies, and GetEgressPolicies

* added baseline service tests and updated logic for unsafe and noselector services with the edgecase of deny all + service no selector in mind

* added more service uts for nodeport and organized scenarios

* updated migration check to be less than 200 characters per line (lint failed)

* updated getExternalTrafficPolicyClusterServices to be less than 200 characters and updated servicesAtRisk to riskSerivces

* removed unused parameter and added edge case scenarios to UTs

* simplified logic

* updated port detection when policy just has a protocol and to flag all egress policies except allow all

* resolved nit: pointer to slice is also a pointer to pointer comment

* responded to comments return false when either port or target port is 0 and print x if there are no selecotr services

* added readme, go mod, go sum, and comments saying why target port will never be undefined

* updated readme

* updated functions using pointers for arrays

* nit changes

* updated with match expressions edgecase

* added uts where target port matches to protocol and port is 0

* added Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster and there are multiple namespaces

* add check for ip no port policies on loadbalancer and fixes label and port logic according to pr comments + UTs to verify

* updated table to use tablewriter

* updated to parse cidr to check for load balancer ip

* removed no selector services from getUnsafeExternalTrafficPolicyClusterServices

* removed noselector services array

* added service selectors to the appended list instead to simplify logic

* Revert "added service selectors to the appended list instead to simplify logic"

This reverts commit 246965d.

* moved checkPolicyMatchServiceLabels check to the top since every block uses it and removed excplicit check for ports

* updated the load balancer health probe ip logic

* added unit tests and logic if nodeport ensure there is no from rules

* removed health probe ip check for loadbalancer services

* added named port check

* nit comment

* reduced output verbosity

* print total number of policies per namespace

* added service and pod count and created a table

* improved formatting

* typo

* updated table format and started to add npm telemetry

* updated verbose flag name and reorganized and removed unused functions

* updated readme

* fixed table formatting

* added ai id and formated tables to be printed after telemetry is sent

* ran tidy

* reduced noise from telemetry runs

* added a const and prefix to metrics

* updated imageVersion per comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

linux npm Related to NPM.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants